Skip to content

Stop and print#191

Open
rouson wants to merge 48 commits into
BerkeleyLab:mainfrom
rouson:stop-and-print
Open

Stop and print#191
rouson wants to merge 48 commits into
BerkeleyLab:mainfrom
rouson:stop-and-print

Conversation

@rouson

@rouson rouson commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@bonachea bonachea left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rouson This PR needs to be rebased and conflicts resolved before CI will run.

I don't have time for an in-depth review today, but here's a few things I noticed on a quick skim

Comment thread test/test_stop_and_print.F90 Outdated
Comment thread src/julienne/julienne_stop_and_print_s.F90
@rouson

rouson commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

@bonachea it appears the all lfortran builds fail, older ifx builds fail, and gfortran 13 fails along. I can't figure out the pattern to the rest of the failures, but I think we'll need to drop support for some of these compiler versions. Let's discuss.

@bonachea

Copy link
Copy Markdown
Member

@bonachea it appears the all lfortran builds fail, older ifx builds fail, and gfortran 13 fails along. I can't figure out the pattern to the rest of the failures, but I think we'll need to drop support for some of these compiler versions. Let's discuss.

I agree with your assessment of the symptoms, but I strongly disagree that we should jump straight to amputation as the only possible treatment. Let's discuss later today.

Comment thread src/julienne/julienne_stop_and_print_s.F90
Comment thread README.md Outdated
Comment thread example/pure-printing/README.md Outdated
Comment thread example/pure-printing/pure-stop-and-print.F90
Comment thread example/pure-printing/write_stuff_m.F90
Comment thread test/modules/character_stop_code_test_m.F90 Outdated
Comment thread test/driver.F90 Outdated

@bonachea bonachea left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed a few missing preprocessor directives that explain some of the current failures.

Comment thread test/driver.F90
Comment thread test/driver.F90

@bonachea bonachea left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and a few more

Comment thread src/julienne_m.F90
Comment thread src/julienne_m.F90

@bonachea bonachea left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the compilers are now passing in CI except LFortran (all of which fail, due to a missing language feature)

Comment thread src/julienne/julienne_command_line_m.f90 Outdated
rouson added 14 commits July 1, 2026 19:18
This commit adds a pure stop_and_print subroutine and a
corresponding unit test.  The new subroutine facilitates printing
string_t objects, including the results of string_t expressions,
during error termination.

Example Usage:

call stop_and_print( "array = " // string_t( [1,2,3,4] ) )
This commit works around several build-time and runtime gfortran
conpiler bugs.
The character_stop_code subroutine is public only for purposes of
calling it from thetest suite so there's no need to have it in the
public interface (julienne_m).
rouson and others added 20 commits July 1, 2026 19:18
Replace `error stop` with  `internal_error_stop`
Co-authored-by: Dan Bonachea <dobonachea@lbl.gov>
Co-authored-by: Dan Bonachea <dobonachea@lbl.gov>
Co-authored-by: Dan Bonachea <dobonachea@lbl.gov>
(cherry picked from commit 07b890f)
This commit defined and uses a new macro to remove the new
stop_and_print feature and corresponding tests with compilers
or compiler versions that do do not compile the code correctly.
Co-authored-by: Dan Bonachea <dobonachea@lbl.gov>
@bonachea

bonachea commented Jul 1, 2026

Copy link
Copy Markdown
Member

Rebased onto the tip of main without changes, to pick-up the CI changes in #194

bonachea added 5 commits July 1, 2026 16:21
This was always crashing at runtime, we just missed the failure in CI
These break all current releases of LFortran, so are being deferred
to a separate PR.

Minor related cleanups
@bonachea

bonachea commented Jul 1, 2026

Copy link
Copy Markdown
Member

@rouson I've pushed the changes we discussed today to remove use of the generic type-bound procedures in command_line_t, which were breaking the Julienne library build on all current releases of LFortran.

I've also added CI coverage for the new stop-and-print example on compilers where we support the feature. This in turn revealed that the feature was actually broken at runtime on GFortran, which I then also repaired.

@rouson

rouson commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

@bonachea apologies if I'm misunderstanding the details. To clarify, I'd like for the generic bindings to exist for users, but I'd be fine with us not using them in the tests. I think we can do this if we drop the tagged releases of lfortran and just use lfortran-latest until there's a new release.

@bonachea

bonachea commented Jul 1, 2026

Copy link
Copy Markdown
Member

I'd like for the generic bindings to exist for users, but I'd be fine with us not using them in the tests.

That's basically what I've deployed in fe9abbb. The generic bindings are still available as command_line_t%generic_argument_present() and command_line_t%generic_flag_value(), they are just not used in any of the tests, which is sufficient to fix the failures on all versions of LFortran (as you can see in CI results for this PR).

I renamed the bindings to minimize the "blast radius" of changes to the existing uses, but if you really feel strongly about the names we can additionally go rename everywhere.

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants